-
Notifications
You must be signed in to change notification settings - Fork 81
AAP-53278 General API optimizations for RBAC #837
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: devel
Are you sure you want to change the base?
Conversation
In terms of the code coverage, that's low because of added test_app to create demo data. I don't want this counted in code coverage, at all. |
7b56ea5
to
9d6746a
Compare
DVCS PR Check Results: PR appears valid (JIRA key(s) found) |
|
attrs['object_id'] = self.initial_data['object_id'] | ||
# If object_ansible_id was provided and converted, use that for object_id | ||
if has_object_ansible_id and not has_object_id: | ||
attrs['object_id'] = attrs['object_ansible_id'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Field Validation Error Causes Semantic Confusion
The ObjectAnsibleIdField
converts the input ansible_id
to an object_id
, storing it in attrs['object_ansible_id']
. The validate
method then incorrectly uses this attrs['object_ansible_id']
(now an object_id
) to determine if an object_ansible_id
was provided, and redundantly assigns it to attrs['object_id']
. This creates semantic confusion and may lead to incorrect validation.
Description
This does general, obligatory, optimizations of API endpoints that were recently added.
The largest code change is because using
LARGE=true
with the bootstrap script wasn't creating all the RBAC objects, additional logic to do so was added. This did help to test cases of >25 Role definitions, role-user-assignments, and role-team-assignments.This work is basically never-ending, so any stopping point is arbitrary. However, the new
/service-index/
views greatly benefit from this.The found solution often repeated for
resource__content_type
also suggests that the resource summary_fields entry is generally not well-optimized, because it's unlikely someone would have continued that to include__content_type
.Type of Change
Note
Optimizes RBAC/service APIs with prefetch/annotation and cached permission lookups, refactors assignment serialization for ansible_id handling, and expands LARGE demo data with roles/permissions plus comprehensive tests.
RoleMetadataView
and pre-warm content types.roledefinition
queryset includesresource
; team/user assignment viewsets useteam__resource
/user__resource
and prefetchresource__content_type
._object_ansible_id_annotation
via subquery (resource_ansible_id_expr
) to avoid N+1.ObjectIDAnsibleIDField
withObjectAnsibleIdField
supporting annotation-based reads and ansible_id→object_id conversion; simplifyBaseAssignmentSerializer
validation/flow forobject_id
vsobject_ansible_id
.created_by
andcontent_type
inEntryReadOnlyViewSet
.content_type_id
inResource.resource_type(_obj)
.roledefinition
toDEMO_DATA_COUNTS
; generate manyRoleDefinition
s withDABPermission
s and assign >25 permissions to users/teams; fix team org assignment to use created org IDs.summary_fields
acrossOrganization
,User
,Team
, andRoleDefinition
.Written by Cursor Bugbot for commit 9d6746a. This will update automatically on new commits. Configure here.